fix: correct evidence_source on findings that claim datasheet provenance they don't have - #37
Conversation
Five findings across two analyzers reported an evidence_source that
contradicts where the value actually came from. Provenance is what tells a
reviewer how much a finding is worth, so a wrong tag is worse than a missing
one -- it launders an API guess or a package average into datasheet evidence.
lifecycle_audit LC-005 (single source) and LC-006 (long lead time) hardcoded
evidence_source="datasheet". Both are derived purely from distributor API
responses; LC-006 reads src_data["lead_time"] and records lead_source="mouser"
two lines above the claim that a datasheet was the evidence. No datasheet
states a lead time. LC-001..LC-004 in the same file already use "api_lookup".
analyze_thermal TH-DET and TS-001/002/003/005 set evidence_source="datasheet"
when rtheta_ja_source == "package_table". That table is
PACKAGE_THERMAL_RESISTANCE -- generic per-package Rtheta_JA averages selected
by a regex over the footprint string. analyze_thermal:394 shows the source is
only ever "package_table" or "default", so there was no branch in which
"datasheet" was correct and the conditional itself was the bug.
lifecycle_audit LT-001 had the mirror defect: it mapped any non-empty
temperature source to "api_lookup", including "extraction_cache", which is a
real per-MPN datasheet extraction. The same analyzer was over-claiming
datasheet provenance in two rules and under-claiming it in a third. Added
_temp_evidence_source(): extraction_cache -> datasheet, api:* -> api_lookup,
everything else (e.g. the "mouser_suggestion" tag at :534) -> heuristic_rule.
Measured on a real 68-component board (24-bit ADC front end, 14 unique MPNs,
DigiKey + Mouser + element14 queried):
before by_evidence_source: {datasheet: 7, api_lookup: 6}
after by_evidence_source: {api_lookup: 13}
trust_level (low), lifecycle_summary (8 active / 6 unknown) and
temperature_summary (8 checked, 0 failing) are all unchanged -- only the
provenance labels move. api_lookup and heuristic_rule are both valid in
finding_schema.py and no key or type changes, so there is no schema drift.
The FastAdd example URL ended in &newcart=true. Per DigiKey's own FastAdd guide the default appends to the current cart while newcart=true starts a new empty one, and the shopping-cart page warns that creating a new cart deletes the items currently in it. An agent following this reference literally destroys whatever the user had already assembled -- and a cart is not recoverable from the API, since DigiKey exposes no cart read/write endpoint. Dropped the parameter from the example, documented it as deliberate opt-in in DigiKey's own wording, and linked the guide. Also stated the "/" separator convention for multiple reference designators. The existing paste example already used C1/C2/C5 but never said why, so it read as cosmetic. _write_digikey_order() in bom_manager.py already rewrites commas to "/", so generated files were never affected; this only matters for hand-assembled paste blocks.
|
Merged, thanks — this is a model bug report. I re-verified everything before merging: the Both defects you deliberately left out are real and now tracked (LC-ACT's missing The FastAdd |
Five findings across two analyzers reported an
evidence_sourcethat contradicts where the value actually came from. Provenance is what tells a reviewer how much a finding is worth, so a wrong tag is worse than a missing one — it launders an API guess or a package average into datasheet evidence.Over-claiming (3 sites)
lifecycle_auditLC-005 (single source) and LC-006 (long lead time) hardcodedevidence_source="datasheet". Both derive purely from distributor API responses — LC-006 readssrc_data["lead_time"]and recordslead_source="mouser"two lines above the claim that a datasheet was the evidence. No datasheet states a lead time. LC-001..LC-004 in the same file already use"api_lookup".analyze_thermalTH-DET and TS-001/002/003/005 setevidence_source="datasheet"whenrtheta_ja_source == "package_table". That table isPACKAGE_THERMAL_RESISTANCE— generic per-package Rθ_JA averages selected by a regex over the footprint string.analyze_thermal:394shows the source is only ever"package_table"or"default", so there was no branch in which"datasheet"was correct and the conditional itself was the bug.Under-claiming (1 site, the mirror defect)
lifecycle_auditLT-001 mapped any non-empty temperature source to"api_lookup", including"extraction_cache"— which is a real per-MPN datasheet extraction. The same analyzer was over-claiming datasheet provenance in two rules and under-claiming it in a third. Added_temp_evidence_source():extraction_cache → datasheet,api:* → api_lookup, everything else (e.g. the"mouser_suggestion"tag at:534) →heuristic_rule.Testing
Validated against
kicad-happy-testharnesson a 40-repo corpus subset (narrower than the recommendedquick_200= 261 repos), producing 382 thermal outputs.A/B,
mainvs this branch, same corpus and same schematic/PCB inputs. Every differing leaf field across all 382 outputs:Nothing else. No temperature, severity, score or count changed, and zero list-length differences — no detection added or removed. Aggregate shift:
{datasheet: 39, geometry: 73, heuristic_rule: 6}→{geometry: 73, heuristic_rule: 45}.Real-world impact: 39 findings and 48 assessments across 40 repos were claiming datasheet provenance for a footprint-regex package average.
Regression assertions (
regression/run_checks.py --type thermal): 9,932 total / 169 passed / 68 failed — identical onmainand on this branch, same failure set. Those 68 are pre-existing drift between the seeded assertions and currentmain, not introduced here.Re-seeding: none needed. No assertion targets
evidence_sourceas a finding field (0 hits for"field": "evidence_source"across 154,664 assertion files — theevidence_sourcekey in those files is the assertion's ownauto_seededprovenance, not the finding's), andbaselines/thermal.jsonrecords only summary counts with no findings array, verified across all 16,062 thermal baselines. No schema drift:api_lookupandheuristic_ruleare both valid infinding_schema.py, and no keys or types change.Not covered by the harness:
lifecycle_audithas no corpus runner (run/has schematic, pcb, gerbers, thermal, emc, spice, datasheets) — it needs network access and API keys. Its three changes were validated separately on a real 68-component board (24-bit ADC front end, 14 unique MPNs, DigiKey + Mouser + element14 queried):by_evidence_source {datasheet: 7, api_lookup: 6}→{api_lookup: 13}, withtrust_level,lifecycle_summaryandtemperature_summaryall unchanged. The newextraction_cache → datasheetbranch is unit-tested over all six source tags but not exercised end-to-end, as that board has nodatasheets/extracted/data.Two related defects found but deliberately left out
These change finding semantics rather than provenance, so they seem like your call rather than mine:
lifecycle_audit:731(LC-ACT) omitsconfidenceandevidence_sourceentirely — this is what forcestrust_level="low"viaunknown_confidence— and writes summary "active" for LCSC-onlyunknownstatus.lifecycle_audit:744(LC-005) —total_queriedcounts only APIs that returned rather than all attempted,status is Nonecounts as active, and LCSC returns no lifecycle status at all, so "only available from X of Y sources checked" is not supported by its own data.Happy to open either as a follow-up.
Second commit (unrelated, docs only)
skills/bom/references/ordering-and-fabrication.md— the FastAdd example URL ended in&newcart=true. Per DigiKey's own FastAdd guide the default appends to the current cart whilenewcart=truestarts a new empty one, and the cart page warns that creating a new cart deletes the items currently in it. An agent following this reference literally destroys whatever the user had already assembled — and it is not recoverable, since DigiKey exposes no cart read/write API endpoint. Dropped the parameter from the example and documented it as deliberate opt-in.Also stated the
/separator convention for multiple reference designators. The existing paste example already usedC1/C2/C5but never said why, so it read as cosmetic._write_digikey_order()inbom_manager.pyalready rewrites commas to/, so generated files were never affected — this only matters for hand-assembled paste blocks.